In general, this is useful. Why is this bad?

how is this different from allow(Article)?

I see that in a spec dedicated to Article it may be strange to mock its methods, specifically the initializer, or other static ones.
However, how do you otherwise test static methods that call other static methods in isolation? (Considering those static methods arr the subject under test, noth the whole class).

Can you probably give a more realistic example of bad code that made you extend this cop in the forst place?

how is this different from allow(Article)?

It's the same as allow(Article), but note that there's another cop RSpec/DescribedClass which would autocorrect Article to described_class.

In fact, I think allow(Article) should be registered as an offense as well since it's also the subject under test (because not everyone might have RSpec/DescribedClass enabled). I can work on this as a separate PR.

In general, this is useful. Why is this bad?

Let's say your subject is subject(:article) { Article.new }. This cop will currently register this:

allow(article).to receive(:foo)

as an offense.

However, it won't register:

allow_any_instance_of(described_class).to receive(:foo)

as an offense, which is quite similar to the above (except the former will stub only one instance, and the latter will stub any instance).

In both scenarios, we are stubbing a method of the object under test.

Can you probably give a more realistic example of bad code that made you extend this cop in the forst place?

Let's say we have a simple module:

module TokenGenerator
  extend self

  def generate
    { token: random_token }
  end

  private

  def random_token
    SecureRandom.uuid
  end
end

and a spec like:

RSpec.describe TokenGenerator do
  before do
    allow(described_class).to receive(:random_token).and_return('123')
  end

  it 'creates a token' do
    expect(TokenGenerator.generate).to eq({ token: '123' })
  end
end

In my view, this is not a good spec because we're not letting the module execute the code we're supposedly testing. A better spec would be:

RSpec.describe TokenGenerator do
  it 'creates a token' do
    expect(TokenGenerator.generate).to include({ token: be_a(String) })
  end
end

which goes through the whole code path of the module.

It’s an easy one.

1. Smells because of stubbing of private methods
2. be_a(String) won’t cut it - if someone mistakenly replaces the call to uuid with a static "not really random", the test won’t catch it. You have to stub, but not the private method, but the external uuid.

Stubbing the class under test’s public methods though may be an indication of a smell or may not. I don’t feel we should always flag corresponding tests as offenses.

Do you have other examples, preferably real life ones?

This is a real-life example 😄

If TokenGenerator were a class instead, this cop would register an offense:

RSpec.describe TokenGenerator do
  subject(:generator) { TokenGenerator.new }

  before do
    allow(generator).to receive(:random_token).and_return('123')
#   ^^^^^^^^^^^^^^^^ RSpec/SubjectStub: Do not stub methods of the object under test.
  end
end

Can you explain to me why should this scenario be an offense under this cop, but the module TokenGenerator example shouldn't?

Fair enough!

But is it still the same "subject under test" if we make assertions on an instsnce, but stub class methods? 🤔

That's a good question. With this PR and also #2094 I've expanded the cop to register offenses for not just the subject's methods (which are typically an instance's methods), but also any method of the class/module for any instance. I guess this gives more responsibility to this cop, which might or might not be desirable.

The style guide shows this:

describe 'Article' do
  subject(:article) { Article.new(author: nil) }

  it 'indicates that the author is unknown' do
    expect(article.description).to include('by an unknown author')
  end
end

as a good example, but the test could still stub the #author method to return nil in any of the following ways:

before do
  allow_any_instance_of(Article).to receive(:author).and_return(nil)
  allow_any_instance_of(described_class).to receive(:author).and_return(nil)
end

and these would not be offenses under any RSpec cop, even though they are in practice similar to:

subject(:article) { Article.new }

before do
  allow(article).to receive(:author).and_return(nil)
end

which is an offense.

Personally, I think (allow|expect)_any_instance_of and described_class should register offenses somewhere, much like RSpec/SubjectStub does, but perhaps this should be a different cop (e.g. RSpec/DescribedClassStub)?

Your argument related to the usage of *_any_instance_of is convincing.

It’s tougher with described_class. I especially like this comment.

It’s tougher with described_class. I especially like rspec/rspec#31 (comment).

True (I also got bit by this when I once used describe :symbol foolishly), but then again, there's the RSpec/DescribedClass cop which enforces usage of described_class, including for stubs. Couldn't offenses for stubbing described_class follow the same logic as that cop?